-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Proposal: Task cancellation shields #3037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
f359bd0 to
a62c1f2
Compare
Introduce a proposal for task cancellation shields to allow certain code to execute regardless of task cancellation status. Implementation WIP PR: swiftlang/swift#85637 update links Updated reference to SE-0493 to include a link.
Co-authored-by: Jamie <2119834+jamieQ@users.noreply.github.com>
Co-authored-by: Jamie <2119834+jamieQ@users.noreply.github.com>
jamieQ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updates seem good. made another pass through and left a number of minor editorial questions/suggestions.
|
|
||
| Child tasks are also affected by task cancellation, and cancellation propagates throughout the entire task tree, allowing for efficient and holistic cancelling of entire hierarchies of work, represented as a tree of child tasks. | ||
|
|
||
| Today, there is no great way to ignore cancellation, and some pieces of code may therefore by accident not execute to completion. This is especially problematic in clean-up or resource tear-down, where a tear-down method's implementation details might be checking for cancellation, however, we _must_ have this code execute, regardless of the task's cancellation status to properly cleanup some resource, like this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today, there is no great way to ignore cancellation
maybe refrain from using 'ignore' here since above it was just stated above that cancellation cannot be ignored. unless that preface was intentional to contrast the existing state with how this feature lets you 'ignore' cancellation in some sense. though still i'm not sure that's the characterization we want to be encouraging.
maybe something along these lines:
Today, there is no way to influence the observation of a Task's cancellation status by code running within it, nor its propagation throughout a task tree, ...
| } | ||
| ``` | ||
|
|
||
| In the above example, while we may have implemented the resource clean-up correctly, we may be unaware of the system only performing actions while the task it is executing in is not cancelled. In order to ensure certain actions execute regardless if called from a cancelled or not cancelled task, we're going to have to "shield" the cleanup code from observing the cancellation status of the calling task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| In the above example, while we may have implemented the resource clean-up correctly, we may be unaware of the system only performing actions while the task it is executing in is not cancelled. In order to ensure certain actions execute regardless if called from a cancelled or not cancelled task, we're going to have to "shield" the cleanup code from observing the cancellation status of the calling task. | |
| In the above example, while the resource clean-up may be implemented correctly, the caller could be unaware that such code may short-circuit if the current task is cancelled. In order for the caller to influence this behavior, it must somehow be able to "shield" the cleanup code from observing the current task's cancellation state. |
|
|
||
| This proposal introduces a new mechanism to temporarily "ignore" task cancellation, called task cancellation shields. | ||
|
|
||
| This can be used to ensure certain pieces of code will execute regardless of the task's cancelled status. A common situation where this is useful is running clean-up code, which must execute regardless of a task's cancellation status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| In the above example, while we may have implemented the resource clean-up correctly, we may be unaware of the system only performing actions while the task it is executing in is not cancelled. In order to ensure certain actions execute regardless if called from a cancelled or not cancelled task, we're going to have to "shield" the cleanup code from observing the cancellation status of the calling task. | ||
|
|
||
| Today, developers work around this problem by creating unstructured tasks, which creates unnecessary scheduling and may have a performance and even correctness impact on such cleanup code: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: since the issues are covered in the list below, maybe just introduce the workaround first rather than repeating them:
| Today, developers work around this problem by creating unstructured tasks, which creates unnecessary scheduling and may have a performance and even correctness impact on such cleanup code: | |
| Today, developers work around this problem by creating unstructured tasks, like this: |
| func example() async { | ||
| let resource = makeResource() | ||
|
|
||
| assert(Task.isCancelled()) | ||
| await Task { | ||
| assert(!Task.isCancelled()) | ||
| await resource.cleanup() | ||
| }.value // break out of task tree, in order to ignore cancellation | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: code didn't compile & the first assertion seems a bit confusing. also, removed the use of 'ignore'
| func example() async { | |
| let resource = makeResource() | |
| assert(Task.isCancelled()) | |
| await Task { | |
| assert(!Task.isCancelled()) | |
| await resource.cleanup() | |
| }.value // break out of task tree, in order to ignore cancellation | |
| } | |
| func example() async { | |
| let resource = makeResource() | |
| await Task { | |
| assert(!Task.isCancelled) | |
| await resource.cleanup() | |
| }.value // break out of task tree, in order to prevent cleanup from observing cancellation | |
| } |
|
|
||
| In order to aid understanding and debuggability of cancellation in such systems, we also introduce a new property to query for a cancellation shield being active in a specific task. | ||
|
|
||
| This API is not intended to be used in "normal" code, and should only be used during debugging issues with cancellation, to check if a shield is active in a given task. This API are _only_ available on `UnsafeCurrentTask`, in order to dissuade from their use in normal code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This API is not intended to be used in "normal" code, and should only be used during debugging issues with cancellation, to check if a shield is active in a given task. This API are _only_ available on `UnsafeCurrentTask`, in order to dissuade from their use in normal code. | |
| This API is not intended to be used in "normal" code, and should only be used during debugging issues with cancellation, to check if a shield is active in a given task. This API is _only_ available on `UnsafeCurrentTask`, in order to dissuade from its use in normal code. |
|
|
||
| // can replicate respecting shield if necessary (racy by definition, if this was queried from outside) | ||
| let isCancelledRespectingShield = | ||
| if unsafeTask.hasTaskCancellationShield { false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an instance method but the above suggests there will only be a static method
| However if a child task (or entire task group) were to be cancelled explicitly, the shield of the parent task has no effect, as it only shields from "incoming" cancellation from the outer scope and not the child task's own status. | ||
|
|
||
| ```swift | ||
| await withTaskCancellationShield { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| await withTaskCancellationShield { | |
| await withTaskCancellationShield { | |
| async let a = compute() // when exiting scope, un-awaited async lets will still be cancelled and awaited |
|
|
||
| Cancellation shielding also prevents the automatic propagation of the cancellation through the task tree. | ||
|
|
||
| Specifically, if a structured child task is created within a task cancellation shield block and the outer task is canceled, the outer task will be canceled. However, we will not observe this flag change until we exit the cancellation shield. At the same time, the child tasks which are running within the task cancellation shield will not become canceled automatically, as would be otherwise the case: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 'canceled' vs 'cancelled' consistency
| Specifically, if a structured child task is created within a task cancellation shield block and the outer task is canceled, the outer task will be canceled. However, we will not observe this flag change until we exit the cancellation shield. At the same time, the child tasks which are running within the task cancellation shield will not become canceled automatically, as would be otherwise the case: | |
| Specifically, if a structured child task is created within a task cancellation shield block and the outer task is cancelled, the outer task will be cancelled. However, we will not observe this flag change until we exit the cancellation shield. At the same time, the child tasks which are running within the task cancellation shield will not become cancelled automatically, as would be otherwise the case: |
| ```swift | ||
| assert(Task.isCancelled) // 🛑 | ||
| withTaskCancellationShield { | ||
| assert(Task.isCancelled == false) // 🟢 | ||
| } | ||
| assert(Task.isCancelled) // 🛑 | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i find this example kind of confusing.
| ```swift | |
| assert(Task.isCancelled) // 🛑 | |
| withTaskCancellationShield { | |
| assert(Task.isCancelled == false) // 🟢 | |
| } | |
| assert(Task.isCancelled) // 🛑 | |
| ``` | |
| ```swift | |
| // assume the current task has been cancelled at this point | |
| print(Task.isCancelled) // true | |
| withTaskCancellationShield { | |
| print(Task.isCancelled) // false | |
| } | |
| print(Task.isCancelled) // true |
Introduce a proposal for task cancellation shields to allow certain code to execute regardless of task cancellation status.
Implementation WIP PR: swiftlang/swift#85637